Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose the disabled property of filters #195

Merged
merged 6 commits into from
Oct 11, 2021
Merged

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Oct 5, 2021

This exposes disabled, as done previously with visible. get_data subsequently ignores filters when they are disabled.

@maximlt
Copy link
Member Author

maximlt commented Oct 6, 2021

The goal with adding/improving the tests was to test that when a widget is disabled it is effectively ignored. I didn't manage in this attempt however to setup a test with a filter/source spec that creates what I wanted.

Copy link
Member

@philippjfr philippjfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just one small change.

@maximlt
Copy link
Member Author

maximlt commented Oct 6, 2021

Oh it almost went in with a default of True for disabled 😨

  • Add a test of get_data with and without a disabled filter

@philippjfr
Copy link
Member

Oh it almost went in with a default of True for disabled 😨

😆 I too missed that. I'll spend some time this week adding at least a basic set of unit tests.

@philippjfr
Copy link
Member

Merging and working on tests separately.

@philippjfr philippjfr merged commit 3d3107e into master Oct 11, 2021
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants